Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Application Crypto cleanup #13746

Merged
merged 9 commits into from
Mar 30, 2023
Merged

Application Crypto cleanup #13746

merged 9 commits into from
Mar 30, 2023

Conversation

davxy
Copy link
Member

@davxy davxy commented Mar 29, 2023

  • Blanket implementation of RuntimeAppPublic trait for any type implementing AppPublic and for which AppPubic::Generic implements RuntimePublic (in practice implements the trait for any application-specific Public that can be used within the runtime).

    • Moving out this from app_crypto! allows to use the macro to implement application specific crypto types that are not necessarily usable by the runtime.
    • This is required for upcoming BLS crypto key types and Sassafras keys as well
  • Blanket implementation of BoundToRuntimeAppPublic trait for any type implementing RuntimeAppPublic

  • Remove unused CRYPTO_ID from RuntimeAppPublic

  • Documentation improvement

  • Relax some trait bounds

@davxy davxy self-assigned this Mar 29, 2023
@davxy davxy added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Mar 29, 2023
@davxy davxy requested a review from a team March 29, 2023 12:46
Copy link
Contributor

@michalkucharczyk michalkucharczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Out of curiosity - why did you remove MaybeHash? Is this because it was not used anywhere? Or maybe it was to restrictive for some type that will be used in this context in future?

@davxy
Copy link
Member Author

davxy commented Mar 29, 2023

lgtm. Out of curiosity - why did you remove MaybeHash? Is this because it was not used anywhere? Or maybe it was to restrictive for some type that will be used in this context in future?

Because I'm dumb 🤦‍♂️
Is required by cumulus. Restoring it

@davxy davxy requested a review from a team March 29, 2023 15:43
@altaua
Copy link
Contributor

altaua commented Mar 29, 2023

This needs to be rebased on current master, there have been breaking changes to the check-crate-publishing job.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

primitives/application-crypto/src/lib.rs Outdated Show resolved Hide resolved
davxy and others added 2 commits March 30, 2023 09:35
@davxy davxy requested review from melekes and a team March 30, 2023 07:35
@davxy davxy merged commit 14f1a39 into master Mar 30, 2023
@davxy davxy deleted the davxy-app-crypto-cleanup branch March 30, 2023 08:30
pgherveou pushed a commit that referenced this pull request Apr 4, 2023
* Adjust application crypto docs

* Blanket implementation for 'RuntimeAppPublic' trait

* Blanket implementation for 'BoundToRuntimeAppPublic' for 'RuntimeAppPublic'

* Relax type bounds

* Docs fix

* restore MaybeHash

* Commit suggestion

Co-authored-by: Anton <anton.kalyaev@gmail.com>

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>
gpestana pushed a commit that referenced this pull request Apr 23, 2023
* Adjust application crypto docs

* Blanket implementation for 'RuntimeAppPublic' trait

* Blanket implementation for 'BoundToRuntimeAppPublic' for 'RuntimeAppPublic'

* Relax type bounds

* Docs fix

* restore MaybeHash

* Commit suggestion

Co-authored-by: Anton <anton.kalyaev@gmail.com>

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Adjust application crypto docs

* Blanket implementation for 'RuntimeAppPublic' trait

* Blanket implementation for 'BoundToRuntimeAppPublic' for 'RuntimeAppPublic'

* Relax type bounds

* Docs fix

* restore MaybeHash

* Commit suggestion

Co-authored-by: Anton <anton.kalyaev@gmail.com>

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants